-
Notifications
You must be signed in to change notification settings - Fork 85
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WIP] - Creates cached state for SubTreeRoots in darksidewalletd #468
Conversation
I had been implementing a solution to #464, apologies for not mentioning this before you took up this work. My PR is #469, please review that. A couple of comments on this PR, the description above says that the subtree roots are returned for a given block height, but they're returned for a given index. Secondly, it doesn't look like you're returning the block hash with each root (probably because
|
Oh! thanks for catching that Larry! |
@LarryRuane I mimicked the models in the GRPC type DarksideSubtree struct {
RootHash []byte
CompletingBlockHash []byte
CompletingBlockHeight uint64
} This will mean that the provided tree states will have to contain the block hash. Should that be ok? |
Well, in production mode, the block hash is returned and was considered an important part of the interface. So I would think that in darkside mode, it should be provided so that we're fully simulating production mode. I was hoping you would agree to close this PR and just go with #469. (But the interface is slightly different, so maybe it's not what you want or need.) |
Idk how I missed that part of the post. Apologize not reviewing yours 🙏 I think this works out. I'm currently updating the Advanced Re Org Tests Regtest datasets to have this new info. This is closed in favor of #469 |
(When completed) Closes #464
I'm following the same reasoning under GetTreeState, let darksidewalletd provide RPC methods
for clients to stage SubTreeRoots when needing to create a spend before sync test scenario
provides:
Please ensure this checklist is followed for any pull requests for this repo. This checklist must be checked by both the PR creator and by anyone who reviews the PR.
As a note, all CI tests need to be passing and all appropriate code reviews need to be done before this PR can be merged